Skip to content

[SYCL] Use %t.dir for DeviceImageDependencies tests #20372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Oct 21, 2025
Merged

[SYCL] Use %t.dir for DeviceImageDependencies tests #20372

merged 3 commits into from
Oct 21, 2025

Conversation

maarquitos14
Copy link
Contributor

Last pulldown pulled the deprecation of %T for llvm-lit, and DeviceImageDependencies tests were using it. It was reported that %t.dir wasn't working for these tests in the pulldown PR (#20343 (comment)), and a workaround was made to make them pass provisionally. This patch moves to %t.dir as it's the standard way after %T removal.

@maarquitos14 maarquitos14 requested a review from a team as a code owner October 15, 2025 18:52
@bader
Copy link
Contributor

bader commented Oct 15, 2025

@maarquitos14, what about this comment: #20343 (comment)?

@maarquitos14
Copy link
Contributor Author

@maarquitos14, what about this comment: #20343 (comment)?

My understanding of that comment is that @aelovikov-intel asked to remove those two commands while we were using the workaround, but now that you bring it up I'm not so sure. Please, @aelovikov-intel, can you confirm?

Actually, I think what's missing is the rm command, but let's wait for Andrei's confirmation.

@aelovikov-intel
Copy link
Contributor

%t.dir should come with rm -rf %t.dir ; mkdir -p %t.dir. We couldn't do the rm part when using %t/.. because that would have affected other tests, but now we should add the rm.

@maarquitos14
Copy link
Contributor Author 

Failed Tests (1):
  SYCL :: SeparateCompile/test.cpp
Failed Tests (1):
  SYCL :: KernelCompiler/sycl_device_globals.cpp

@intel/llvm-gatekeepers I think these should be unrelated. My PR is only changing 4 SYCL E2E tests, which are none of these. Can this be merged?

@aelovikov-intel
Copy link
Contributor

aelovikov-intel commented Oct 20, 2025 •
edited
Loading

SYCL :: SeparateCompile/test.cpp is known, the other one is also known: #19965

@steffenlarsen steffenlarsen mentioned this pull request Oct 21, 2025
Open
@steffenlarsen
Copy link
Contributor

SYCL :: KernelCompiler/sycl_device_globals.cpp on CUDA reported in #20408.

@steffenlarsen steffenlarsen merged commit a31aa2c into intel:sycl Oct 21, 2025
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aelovikov-intel aelovikov-intel aelovikov-intel approved these changes

@cperkinsintel cperkinsintel Awaiting requested review from cperkinsintel cperkinsintel is a code owner automatically assigned from intel/llvm-reviewers-runtime

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maarquitos14 @bader @aelovikov-intel @steffenlarsen